Conversation
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Pull request overview
This PR introduces initial ARC ↔ TCKDB integration by adding a post-run upload sweep (driven by output/output.yml), TCKDB payload/sidecar writing + idempotency, and several supporting schema/provenance enhancements across ARC output, parsers, and job execution.
Changes:
- Add a TCKDB upload sweep (species/reaction modes), CLI re-runner, payload sidecars, and idempotency-key utilities.
- Extend ARC output/provenance to support richer thermo points, TS path-search artifacts (NEB/GSM), IRC direction tracking, and constraints.
- Improve operational robustness (Arkane stderr handling, SSH pooling, remote file handling, TS adapter filtering) and update documentation.
Reviewed changes
Copilot reviewed 65 out of 69 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/tckdb_upload_boundary.md | Defines ARC→TCKDB semantic boundary and guardrail expectations. |
| docs/output_yml_schema.md | Updates output.yml schema docs (thermo_points, GSM log). |
| docs/gaussian.md | Adds Gaussian run/troubleshooting notes (new doc). |
| arc/testing/test_JobAdapter/calcs/Species/spc1/opt_a472/submit.sh | Adds PBS submit fixture for JobAdapter tests. |
| arc/testing/test_JobAdapter/calcs/Species/spc1/opt_a472/input.gjf | Adds Gaussian input fixture for JobAdapter tests. |
| arc/testing/test_JobAdapter/calcs/Species/spc1/opt_a1313/submit.sh | Adds PBS submit fixture for JobAdapter tests. |
| arc/testing/test_JobAdapter/calcs/Species/spc1/opt_a1313/input.gjf | Adds Gaussian input fixture for JobAdapter tests. |
| arc/testing/test_JobAdapter/calcs/Species/spc1_and_2_others/conf_opt_a472/submit.sh | Adds PBS submit fixture for multi-species JobAdapter tests. |
| arc/testing/test_JobAdapter/calcs/Species/spc1_and_2_others/conf_opt_a472/input.gjf | Adds Gaussian input fixture for multi-species JobAdapter tests. |
| arc/testing/test_JobAdapter/calcs/Species/spc1_and_2_others/conf_opt_a1313/submit.sh | Adds PBS submit fixture for multi-species JobAdapter tests. |
| arc/testing/test_JobAdapter/calcs/Species/spc1_and_2_others/conf_opt_a1313/input.gjf | Adds Gaussian input fixture for multi-species JobAdapter tests. |
| arc/testing/test_JobAdapter_ServerTimeLimit/calcs/Species/spc1/opt_101/err.txt | Adds PBS walltime-limit stderr fixture. |
| arc/testing/test_JobAdapter_scan/calcs/Species/methanol_and_5_others/scan_a472/input.gjf | Adds Gaussian scan input fixture. |
| arc/testing/test_JobAdapter_scan/calcs/Species/methanol_and_5_others/scan_a1313/input.gjf | Adds Gaussian scan input fixture. |
| arc/tckdb/sweep.py | Implements end-of-run sweep dispatch and artifact uploading. |
| arc/tckdb/payload_writer.py | Writes payload JSON + sidecar metadata; replay gating helper. |
| arc/tckdb/payload_writer_test.py | Adds unit tests for payload/sidecar writing and replay gate. |
| arc/tckdb/idempotency.py | Adds idempotency-key builders for payloads and artifacts. |
| arc/tckdb/idempotency_test.py | Adds unit tests for idempotency-key stability/distinctness. |
| arc/tckdb/constraints.py | Adds constraint dataclass + serializer for TCKDB payloads. |
| arc/tckdb/config.py | Adds TCKDB config parsing, API-key resolution, artifact-kind validation. |
| arc/tckdb/cli.py | Adds standalone CLI to rerun sweep against existing project output.yml. |
| arc/tckdb/cli_test.py | Adds tests for CLI parsing and dispatch behavior. |
| arc/tckdb/init.py | Exposes adapter/config public API. |
| arc/statmech/arkane.py | Unifies tunneling method constant; fixes Arkane success signal to output.py existence; maps thermo_points. |
| arc/statmech/arkane_test.py | Adds tests for Arkane stderr classification + output.py success gating. |
| arc/species/species.py | Ensures monoatomic species get a trivial final_xyz; adds rotor scan software field; renames cp_data→thermo_points in ThermoData. |
| arc/species/species_test.py | Adds tests for monoatomic final_xyz behavior; updates xyz fixtures expectations. |
| arc/settings/submit.py | Clarifies example submit templates; adds ORCA auxiliary file copies; renames example keys and adds back-compat aliases. |
| arc/settings/submit_test.py | Updates tests to validate example keys/aliases without restricting user overrides. |
| arc/settings/settings.py | Updates ORCA NEB default level; expands candidate RMG install paths. |
| arc/scripts/save_arkane_thermo.py | Emits richer thermo_points (Cp/H/S/G) instead of cp_data. |
| arc/scripts/get_species_corrections.py | Adds helper script to compute AEC/BAC totals/components via Arkane in RMG env. |
| arc/scripts_test.py | Updates tests for thermo_points extraction. |
| arc/scheduler.py | Adds TS-guess path routing (NEB vs GSM), IRC direction tracking, TS adapter filtering, remote cleanup hook, and scan software tracking. |
| arc/scheduler_test.py | Adds tests for TS guess paths routing and GSM path key seeding. |
| arc/reaction/reaction_test.py | Adds regression test for opt-done gate with monoatomic reactant. |
| arc/parser/parser.py | Adds rich IRC parser wrapper, scan absolute energies wrapper, GSM trajectory filename handling. |
| arc/parser/parser_test.py | Adds tests for rich IRC parsing behavior. |
| arc/parser/constraints_test.py | Adds tests for Gaussian/ORCA constraint parsing. |
| arc/parser/adapters/orca.py | Adds ORCA input constraint parser. |
| arc/parser/adapter.py | Adds default hook for absolute scan energies in Hartree. |
| arc/job/trsh.py | Adds “DispUnconverged” detection and no_tight troubleshooting path. |
| arc/job/trsh_test.py | Adds tests for displacement-only unconverged detection. |
| arc/job/ssh.py | Adds delayed-existence retry for downloads; improves job-id parsing; SSH connect key handling; adds remove_dir. |
| arc/job/ssh_pool.py | Adds process-lifetime SSH connection pool. |
| arc/job/pipe/pipe_coordinator.py | Prevents pipe mode for engines resolving to remote servers. |
| arc/job/pipe/pipe_coordinator_test.py | Adds test ensuring pipe is disabled when engine resolves to remote server. |
| arc/job/adapters/ts/xtbgsm_test.py | Adds tests for GSM provenance capture and ograd preservation behavior. |
| arc/job/adapters/ts/xtb_gsm.py | Records GSM stringfile path as TSGuess log provenance; tracks node-output dir. |
| arc/job/adapters/ts/orca_neb.py | Makes NEB input refer to local reactant/product xyz filenames (not absolute). |
| arc/job/adapters/scripts/xtb_gsm/ograd | Preserves per-node energy/gradient/xtbout files for provenance. |
| arc/job/adapters/gaussian.py | Applies no_tight troubleshooting flag to drop opt=tight. |
| arc/job/adapter.py | Adds SSH reuse (pool/shared) across upload/submit/download; submit-script validation; remote cleanup; HTCondor job.log gating. |
| ARC.py | Wires TCKDB sweep into main execution and closes SSH pool on exit. |
| ARC_test.py | Adds tests intended to cover ARC.py/TCKDB sweep wiring. |
| .gitignore | Adds ignores for agent folders and ARC.egg-info. |
Comments suppressed due to low confidence (1)
docs/gaussian.md:33
- In the Gaussian input template,
MULTIPLICTYis misspelled; this should beMULTIPLICITYto avoid confusion for users copying the template.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # We treat both "ts_label set but TS missing/non-converged" and | ||
| # "no ts_label at all" as the partial case. The latter is rare | ||
| # (ARC normally emits a ts_label even on failure), but a | ||
| # reaction record with reactants/products and no TS is exactly | ||
| # the partial-shape we want to allow under the flag. | ||
| is_partial = bool(ts_label) and not ts_converged | ||
|
|
| ## Troubleshooting | ||
|
|
||
| There are many situations where we can gaussian job errors and gaussian in the output file reports the error code. | ||
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #890 +/- ##
==========================================
+ Coverage 63.13% 64.65% +1.52%
==========================================
Files 113 122 +9
Lines 37970 41380 +3410
Branches 9955 10670 +715
==========================================
+ Hits 23971 26753 +2782
- Misses 11126 11508 +382
- Partials 2873 3119 +246
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
239423f to
ee0be10
Compare
Freq calculations now carry a HessianPayload so TCKDB's calculation_hessian table is populated on upload. - arc/parser/adapters/gaussian.py, orca.py: parse_cartesian_hessian_lower_triangle() returns the packed lower triangle (with diagonal, row-major) in NATIVE Hartree/Bohr^2 — the same blocks Arkane reads (Gaussian "Force constants in Cartesian coordinates:" needs iop(7/33=1), already set on ARC freq jobs; Orca sibling input.hess $hessian block), minus Arkane's J/m^2 conversion. Returns None (never raises) when absent / monatomic / malformed. - arc/tckdb/adapter.py: attaches hessian to the freq calculation, geometry-bound to the freq input geometry, source=parsed_log/parsed_hess; wrapped so it can never break payload construction. - Tests: parser fixtures (Gaussian NH/CHO, new Orca H2O .hess), triangle-length invariant, native-unit sanity, absent-path, and HessianPayload validation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A transition state's statmech harmonic_frequencies_cm1 lists only the real modes; the imaginary (reaction-coordinate) frequency is carried separately in imag_freq_cm1. The adapter set is_imaginary = freq < 0 over the real-only list, so it emitted n_imag=1 with zero imaginary modes — which TCKDB's FreqResultPayload validator (count(is_imaginary) must equal n_imag) rejects. Re-insert imag_freq_cm1 as a negative imaginary mode so modes are internally consistent with n_imag; if they can't be reconciled (e.g. a higher-order saddle with a single stored imag_freq_cm1), omit modes rather than emit a payload the backend rejects. Surfaced by an end-to-end upload of a real ARC project against a live TCKDB instance. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a TS did not converge, its spawned IRC endpoint species (e.g. IRC_TS0_1) is deleted. The scheduler removed it from self.output and self.species_dict, but removed it from self.species_list via reassignment (self.species_list = [...]), which rebinds only the scheduler's variable and leaves the deleted species dangling in ARC.species (the same list object, mutated in-place by the earlier append). save_project_info_file then iterates ARC.species and looks each up in the synced self.output, raising KeyError and crashing the run at the final summary step after all work was done. Fix: (1) delete in-place via slice assignment so removals propagate through the shared list like the appends do; (2) guard the summary loop to skip any species missing from self.output (deleted IRC species shouldn't be reported anyway). Adds a regression test.
Follow-up to the IRC-species KeyError fix: save_project_info_file has a SECOND loop (building <project>_info.yml for T3) with the same unguarded self.output[species.label] lookup, so a dangling deleted IRC species would KeyError there as well once the first loop was guarded. Guard it identically (skip species not in self.output). Also rewrite the regression test to exercise save_project_info_file on a bare ARC instance (ARC.__new__ + SimpleNamespace stand-ins) instead of constructing a full ARC object. The fix is pure bookkeeping and does not need the Arkane BAC tables / molecule perception that a full ARC build pulls in; the test now runs in ~1s instead of minutes and has no RMG dependency.
In computed-reaction upload mode, run_upload_sweep dispatches only the reaction sweep, so species reach TCKDB solely inline inside each reaction bundle. When a reaction is partial (TS missing/non-converged), that bundle either is skipped (allow_partial_uploads=false) or written as a .partial sidecar that never live-POSTs — so a reaction with a failed TS but individually-converged reactant and product species uploaded no species at all. Converged minima were lost with the failed reaction. Fix: for a partial reaction, salvage each reactant/product species whose own `converged` flag is true (the same eligibility gate _run_species_sweep uses) via the existing computed-species path (SPECIES payload shape). Runs before the allow_partial_uploads branch, so species are salvaged whether the reaction record is skipped or sidecarred — allow_partial_uploads governs only the reaction/kinetics provenance record, not converged minima. TS and kinetics are never salvaged. Non-converged species are skipped. A sweep-wide seen-set dedups a species shared across partial reactions; fully-converged reactions are unchanged (species still ship inline, no double upload).
…by enum)
The backend hoists parameters_json.tckdb_origin.origin_kind into the validated
CalculationWithResultsPayload.origin_kind enum {executed, reused_result, imported,
derived} — proven by reused_result round-tripping while screened_conformer 422s
under identical nesting. So parameters_json is not opaque for this key; the
"free-form" assumption was wrong.
_screened_conformer_origin (alt/screened conformer opt rows: converged geometry +
relative energy from the conformer screen, no independently executed ESS job) emitted
origin_kind='screened_conformer', which is not an enum member, so every computed-species
bundle containing an alt conformer 422'd. Map it to 'derived' (a screened conformer's
geometry is derived from the conformer screen, not independently executed) and preserve
the ARC-specific 'screened_conformer' distinction verbatim under a new origin_detail key
(backend-opaque, like reason/producer). The selected conf0 is unmarked (real executed
opt) and unaffected; reused_result rows unchanged.
Add VALID_TCKDB_ORIGIN_KINDS + schema-conformance guard tests that walk each generated
payload and assert every origin_kind (at any depth) is a valid enum member, so this
class of 422 cannot recur silently.
…stion A transient TCKDB server not-ready state during a long ARC run made the in-run upload fail its one-shot /readyz preflight, dropping the reaction upload (payload was written; a manual CLI re-run succeeded once the server was up). _ensure_ready now retries the probe up to PREFLIGHT_MAX_ATTEMPTS (5 = 1 initial + 4 retries) with exponential backoff (PREFLIGHT_BASE_DELAY_SECONDS * 2**i, capped at PREFLIGHT_MAX_DELAY_SECONDS -> 1,2,4,8s, ~15s worst case), retrying both failure modes (a TCKDBError request exception and a 200 body that reports not-ready), and returns on the first ready response. The single-check-per-adapter-instance contract is preserved (the loop runs once; the final error is cached in _preflight_error). Backoff sleeps route through _preflight_sleep so tests patch them out. On genuine exhaustion, _record_failure now calls _log_readiness_recovery for a TCKDBReadinessError, logging that payloads were written and the exact copy-pasteable re-run command (python -m arc.tckdb.cli <input.yml> -p <project> --upload-mode <mode>) using the real project dir / upload mode in scope.
Fills the "TS-specific adapter not yet implemented" gap: converged transition states are no longer deferred — a new computed_ts upload mode composes a TransitionStateUploadRequest and POSTs each converged TS to /uploads/transition-states. - adapter.py: submit_computed_ts_from_output + _compose_transition_state_request + _ts_calc_to_standalone + _build_ts_reaction_upload. Reuses _build_ts_block (results stay wrapped as opt_result/freq_result/... = the CalculationWithResultsPayload shape), the geometry/identity fragments, correction schemes, origin markers, and idempotency. primary_opt is the type=opt calc (backend validator); additional_calculations carry freq/sp/irc/path_search. Bundle-only keys stripped for the extra=forbid fragments. - sweep.py: _run_ts_sweep uploads each transition_states entry whose own `converged` flag is true (same gate as species), maps TS->reaction via ts_label, skips non-converged and TSs with no referencing reaction (never fabricates a reaction). - config.py: UPLOAD_MODE_COMPUTED_TS; CLI picks it up from VALID_UPLOAD_MODES. Review (Fable) validated a REAL reaction_05 payload against the REAL backend pydantic schema on the Pi (construction only, no POST): primary_opt=opt, freq/sp/irc attached, origin_kind valid, no leaked keys. Sub-critical fixes applied: standalone path never builds/inlines artifacts (backend has no slot) and warns once if artifacts.upload is set; applied_energy_corrections omission documented + debug-logged; whitespace-only reaction_family gated out (mirrors backend normalization) to avoid a 422. Follow-up (documented): replace the test's reconstructed TransitionStateUploadRequest with a real tckdb_schemas import once the package publishes it.
The GSM stringfile has no per-frame energies (comment lines are all 0.000000,
verified across every benchmark stringfile), so path points were geometry-only.
Rather than fabricate a flat-zero energy profile, this populates path_coordinate
correctly and adds a guarded energy fallback for GSM builds that DO emit energies.
- parser.py: parse_gsm_stringfile_energies() — reads the per-frame comment energy
(relative kcal/mol) or None; parse_trajectory untouched (its callers unaffected).
- adapter.py _build_path_search_result_payload:
* path_coordinate = cumulative Kabsch-ALIGNED inter-node displacement in Angstrom
(the analog of ORCA NEB's Dist.(Ang.) the backend column stores). GSM does not
keep every node in a common frame — the reactant endpoint is often translated/
rotated vs the interior, which would inflate a raw displacement (~1.56 A vs
~0.15 A aligned for node 0->1) — so each node is aligned onto its predecessor
(converter.kabsch) before summing. Node 0 = 0.0; null everywhere if alignment
fails. This replaces an earlier index/(N-1) fraction, which mislabeled a shared
column whose ORCA rows hold Angstroms.
* guarded stringfile relative-energy fallback: only when NO per-node Hartree was
preserved AND the comment column spans > 1e-6 kcal/mol (not the all-zero
sentinel), avoiding a fabricated flat profile and source-mixing with partial
Hartrees; flags the peak is_climbing_image.
The real per-node energy gap is producer-side (GSM stdout / gsm_node_outputs not
preserved on the run) and tracked separately. Parser=stays / adapter=extractable.
Introduces a `method_sources` attribute to the TSGuess class. This attribute stores all methods that produced an equivalent xyz guess. Normalizes the method sources to a unique, ordered, lowercase list, ensuring consistency and avoiding duplicates. This allows for better tracking of the origins of TS guesses and simplifies the clustering logic.
… populated Level.as_dict() used all(self.args.values()), which dropped the entire args entry whenever either 'keyword' or 'block' was empty -- the common case for users who pass only keyword args. The scheduler reconstructs the level via Level(repr=level) before handing it to the job adapter, and re-feeds job.level into run_job during trsh, so the loss surfaced as user-supplied keywords (e.g. opt=(verytight)) silently disappearing on every trsh re-run while the initial run worked correctly. Switch to any() so args is preserved when at least one slot has content; still dropped when both are empty.
…pters TSGuess now records the level of theory at which the guess-generating adapter ran its electronic structure calculations, as a plain YAML-safe dict that round-trips through as_dict()/from_dict(): - orca_neb: the adapter's NEB level (from job args or orca_neb_settings) - xtb_gsm: the job level if set, else GFN2-xTB (the ograd script runs plain 'xtb --grad', i.e., the xtb default) - crest: GFN2-xTB (CREST is run without an explicit method flag) Pure ML/template adapters (heuristics, GCN, AutoTST, KinBot, linear) leave it None. Adds a plain_level_dict() helper to arc.level for the shared Level-to-plain-dict serialization.
1+2_Cycloaddition, Diels_alder_addition, Retroene, XY_elimination_hydroxyl, and intra_halogen_migration only had linear (or kinbot+linear), which finds spurious H-motion saddles that NMD correctly rejects. Adds the double-ended path finders so the real heavy-atom TS can be located (same fix as XY_Addition).
…A NEB level
Two issues surfaced running the QST2 adapter (reaction 21, intra_halogen_migration):
1. process_completed_tsg_queue_jobs hard-coded method='orca_neb' for every .log queue TS-guess
job. Now that more than one queue adapter emits a .log (orca_neb and qst2), QST2's guess was
collected but mislabeled 'orca_neb'. Thread the actual adapter through: the method now takes a
`method` argument and the scheduler passes job.job_adapter.
2. orca_neb_settings['level'] used 'wb97xd/def2tzvp', but ORCA rejects Gaussian's 'wb97xd' spelling
("UNRECOGNIZED KEYWORD WB97XD"), so every NEB job errored and produced no geometry to parse
("Fallback parsing failed"). Use the ORCA spelling 'wb97x-d3/def2tzvp'.
Adds a regression test asserting a .log queue guess is attributed to 'qst2', not 'orca_neb'.
GSM's driver invokes the copied ograd gradient wrapper as ./ograd, which
requires the executable bit. write_input_file (incore) and set_files (queue)
copied ograd without +x (safe_copy_file/copyfile drops mode, tracked template
was 100644), so ograd landed 644 and GSM aborted with permission denied.
- incore: add change_mode('+x') on self.ograd_path
- queue: pass make_x=True on the ograd upload dict
- git: make the tracked ograd template 100755 (defense-in-depth)
- test: assert incore ograd is X_OK; flip set_files ograd expectation to make_x=True
When ARC's TS-guess deduplication (ARCSpecies.cluster_tsgs) merges several guesses with the same geometry into one, it keeps a single primary `method` plus a merged `method_sources` list. Path-search provenance (GSM/NEB per-node energies and points) keyed off the SINGLE primary method, so when a geometry-only method (e.g. gcn) won dedup over an absorbed path-search source (xtb-gsm / orca_neb), the GSM/NEB log was dropped: `paths['gsm']` was never set in the scheduler and the TCKDB `path_search` calc was skipped. Additive fix (does NOT change which guess/method wins selection): - TSGuess gains `method_source_paths` (method -> log path). cluster_tsgs seeds it from each clustered guess's live method/log_path — the xtb_gsm/orca_neb/ qst2 adapters assign log_path *after* construction, so the dict is empty at clustering time and must be seeded there, not only in __init__. Persisted through as_dict/from_dict for restarts. - scheduler._ts_guess_path_provenance: primary method first, else the first path-search method in method_sources with a preserved log. Used at both output[label]['paths'] write sites. - output.py restart-fallback recovers a merged source's log into gsm_log/ neb_log, single-slot (first source wins) to match the scheduler. - tckdb adapter._resolve_ts_guess_path_search_for_record gates emission off the chosen guess's method_sources, preferring the method whose log field is populated so the gate and log lookup agree for multi-source merges.
…ints The xtb-GSM TS search writes per-node xTB outputs into gsm_node_outputs/ (0000.NN.xtbout), each carrying a real total energy, but those energies never reached the TCKDB path_search_result.points: the reader globbed *.energy files, while the patched ograd wrapper reliably preserves only *.xtbout (the .energy/.gradient copies depend on a tm2orca.py rename that usually doesn't land). So electronic_energy_hartree / relative_energy_kj_mol came out null despite the data existing on disk. - Add _parse_xtb_xtbout_energy: robustly parse the xTB total energy (Hartree) from an .xtbout file (accepts either the ':: total energy .. Eh' block or the boxed '| TOTAL ENERGY .. Eh |' banner, case-insensitive, last-match-wins; UTF-8 with errors='replace' so the non-ASCII 'Eh/α' glyph can't raise UnicodeDecodeError and abort the bundle build). - _read_gsm_node_outputs now prefers the cleaner Turbomole .energy file (with gradient), then falls back to .xtbout (energy only) for any node still missing. - Relax the relative-energy rule from strict all-or-none to: reference every point that has an absolute energy to min(known); points without one stay explicit nulls. Real GSM stringfiles have N+1 frames but only N node-energy files (the fixed reactant-anchor frame 0 has no ograd energy), so strict all-or-none suppressed the whole profile on all real data. Node label NN maps to stringfile frame index NN; frame 0 is the orphan. Confirmed on reaction_06: 14 energies parse, the -9.0206 Eh peak (node 8) lands exactly on the selected TS-guess frame, relatives reference the reactant well. Tests: .xtbout parsing (formats, garbled/missing -> None, non-UTF-8 bytes, Eh-unit gating, last-match), reader xtbout fallback + .energy precedence + mixed sources, and the real N+1-frame orphan-endpoint end-to-end shape.
…ilure compare_thermo ran the RMG/Arkane thermo script via execute_command, which returns stderr as a list of lines and does not surface the return code. RMG/Arkane write their normal INFO:root/WARNING:root startup logging to stderr even on a fully successful run, so `if len(stderr): logger.error(...)` emitted a false "Error while running RMG thermo script" on every reaction, despite output/RMG_thermo.yml being produced and consumed. It was a logger.error only (never raised), so the impact was cosmetic noise in arc.log. Fix, kept local to compare_thermo (execute_command's signature is unchanged): - Filter stderr, dropping benign `^(INFO|WARNING|DEBUG):root` lines and blanks; demote that chatter to logger.debug. - Only logger.error if real (non-log) error content remains OR the deliverable wasn't produced (gate on RMG_thermo.yml yielding species with h298/s298, since the return code isn't available here). A genuine failure (real traceback, or an unpopulated/missing deliverable) still logs an error. - Early-return when there are no species to compare (avoids a pointless RMG database load and a spurious error report). Adds focused unit tests: benign INFO/WARNING stderr + populated deliverable logs no error; a real traceback, and separately an unpopulated deliverable, each still log an error.
The backend StatmechInBundle / BundleStatmechIn now accept optical_isomers (int, ge=1); ARC already carries the value in output.yml (species[*].statmech.optical_isomers, from Arkane's conformer.opticalIsomers) but _build_statmech_block_for_species dropped it, copying only external_symmetry. Add the field with the same int>=1 guard so absent/non-int/0 are omitted. The shared builder covers both computed-species and computed-reaction bundle modes.
PayloadWriter.write atomically overwrote the canonical <label>.payload.json and its .meta.json sidecar on a re-run for the same label, destroying the prior payload plus the sidecar that held the last upload's status/idempotency_key/server IDs. The TCKDB backend appends on re-upload (old + new bundles resolved in review), so destroying the local prior bundle broke the trail mapping which local run produced which DB record. Before overwriting, move the existing payload+sidecar pair into an archive/ subdir under a self-describing name keyed off the prior sidecar's identity: uploaded_at if present (the accepted-record moment), else idempotency_key (the server's dedupe key), else the payload mtime, else a monotonic counter. The key is made filesystem-safe (colons stripped). Colliding archive names are disambiguated with a numeric suffix. Move-then-write ordering means a crash can never lose the prior payload (it is either canonical or archived, never gone). First writes archive nothing. Default-on via archive_previous=True; existing callers untouched. The canonical current path is unchanged, so upload/replay/discovery tooling is unaffected (no in-repo tooling globs the payload dirs; archive/ is a nested subdir external discovery must skip).
Open-shell/unrestricted single points carry an <S**2> spin-contamination
value that is needed to replicate/validate Arkane thermo for radicals and
biradicals. ARC parsed no such diagnostic and output.yml carried none, so
the TCKDB backend's ready spin_diagnostic slot went unfilled.
Parser: add parse_s_squared to the ESS adapter interface (default None on
the base class) with implementations for Gaussian (converged post-SCF
<S**2>=, plus "S**2 before annihilation ... after" -> s_squared_annihilated,
expected S(S+1) from the log's multiplicity), ORCA ("Expectation value of
<S**2>" + "Ideal value S*(S+1)" -> expected; no annihilation), and QChem
(<S^2> =; expected from the echoed $molecule multiplicity). Registered as
parser.parse_s_squared; s_squared_expected_from_multiplicity helper added.
Restricted/closed-shell logs print no <S**2> -> None.
output.yml: _parse_spin_diagnostic parses the sp log (falling back to
freq/opt when the sp energy reused the opt output), recomputing the ideal
S(S+1) from ARC's authoritative multiplicity; _spc_to_dict writes it as
sp_spin_diagnostic (None for closed-shell).
TCKDB emit: _spin_diagnostic_payload builds SpinDiagnosticPayload and
attaches it as the sp calc's spin_diagnostic in _calculation_payload (the
single funnel for all sp construction sites), only when s_squared was
parsed; closed-shell/absent -> block omitted (never all-null). ge=0 and the
required s_squared field are respected; optional companions omitted when
absent. Coupled-cluster wavefunction_diagnostic (T1/D1) deferred.
Tests: parser unit tests (Gaussian doublet/triplet, ORCA, QChem, closed-
shell None, malformed None) against real repo fixtures; output.yml plumbing
tests; TCKDB emit tests incl. full CalculationWithResultsPayload
model_validate against installed tckdb-schemas.
…ions) For an A+A reaction (two reactants that are the same species, e.g. OH + OH), Arkane's own save_thermo_lib raises DatabaseError on the R1==R2 duplicate at the very end of its run -- after output.py is written but before RMG_libraries/thermo.py is. ARC's save_arkane_thermo.py then returned early (it guards on that missing library file), so thermo.yaml was never written and parse_arkane_thermo_output left spc.thermo null: null thermo in output.yml and an empty project thermo library, despite Arkane having computed full NASA polynomials. Two independent fixes: 1. Robust reload (primary): when RMG_libraries/thermo.py is absent, save_arkane_thermo.py now reconstructs each species' NASA thermo directly from output.py (evaluated with the same rmgpy classes Arkane uses to read these files back) and produces thermo.yaml exactly as the library path would. This recovers thermo from ANY save_thermo_lib failure, not just A+A. Happy path unchanged: the library file is used when present. 2. Dedup at the source (secondary): _dedup_thermo_species_list collapses species with identical adjacency list + multiplicity into a single Arkane thermo block (full-thermo run only; kinetics and e0-only keep every block), so save_thermo_lib never sees a duplicate and never crashes. The collapsed duplicate label's thermo (and E0/symmetry) is copied from the representative in parse_arkane_thermo_output. The computed_reaction TCKDB payload was unaffected (it carries no NASA). Tests: A+A dedup + propagation and an all-distinct regression (arc_env); output.py reconstruction and library-precedence (rmg_env, run as a script).
# Conflicts: # .gitignore
| if len(tokens) >= 2: | ||
| try: | ||
| multiplicity = int(tokens[1]) | ||
| except ValueError: |
| sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) # for 'common' and the module under test | ||
|
|
||
| try: | ||
| from rmgpy.thermo import NASA # noqa: F401 |
| """ | ||
|
|
||
| import copy | ||
| import unittest |
A coordinate-less TS guess (e.g. a failed kinbot/queue guess, or a queue TS-search job whose .log produced no parseable geometry) reached almost_equal_coords(None, None) during cluster_tsgs, raising a TypeError that aborted the entire scheduler at init (benchmark reaction_08). - almost_equal_tsgs: if either guess's get_xyz() is None, return False instead of comparing coordinates. A guess with no geometry can never be a geometric duplicate. - process_completed_tsg_queue_jobs: when a queue .log yields no usable geometry, mark the guess failed and log a warning instead of silently dropping it, so it is never kept as a clusterable 'successful' guess. Adds regression tests covering both paths.
Register 'Disproportionation' in both TS-adapter gates for autotst: the adapter's own supported_families and ts_adapters_by_rmg_family. intra_H_migration was already enabled and is untouched. Note: Disproportionation requires the AutoTST env on a branch whose SUPPORTED_FAMILIES includes it (e.g. fix/disproportionation-support), not AutoTST main. Adds a focused test asserting both gates admit it.
| shutil.rmtree(path, ignore_errors=True) | ||
| try: | ||
| os.rmdir(self.project_dir) | ||
| except OSError: |
An impossible spin multiplicity (e.g. an even multiplicity for a species
with an even electron count) was forwarded all the way to the ESS, where
it surfaced as a cryptic non-retryable Gaussian GL301 InputError and
aborted the species after wasting a full run.
Enforce the parity relation
(total_electrons - net_charge) % 2 == (multiplicity - 1) % 2
in ARCSpecies once charge and multiplicity are finalized, raising a clear
SpeciesError that names the label, electron count, net charge, requested
multiplicity, and the nearest valid multiplicities. This is an inviolable
parity relation, so the guard has zero false positives. It is skipped
gracefully when the electron count is not yet known (e.g. a TS or an
xyz-less species with no perceived Molecule).
This pull request introduces several improvements and new features to ARC, focusing on TCKDB integration, SSH connection pooling for remote jobs, and more robust test isolation. The most significant changes are outlined below.
TCKDB Integration and Upload Sweep Enhancements:
ARC.py, including configuration loading, conditional activation, and a post-run upload sweep viarun_upload_sweep. This enables automated uploading of results to TCKDB after a run. [1] [2]ARC_test.pyfor the TCKDB upload sweep dispatcher, covering dispatch logic, error handling, summary output, and artifact path resolution.SSH Connection Pooling for Remote Jobs:
arc/job/adapter.pyto use a process-global SSH connection pool for remote-queue jobs, reducing the number of SSH connections by reusing a single connection per server for the duration of a run. This includes new logic for borrowing and releasing SSH clients, and a helper for managing SSH context.Test Isolation and Local Overlay Control:
arc/imports.pyto allow disabling user-local settings overlays (e.g.,~/.arc/settings.py) during tests or via theARC_IGNORE_LOCAL_SETTINGSenvironment variable. This ensures tests run with consistent settings and are not affected by user-specific configurations. [1] [2] [3]These changes collectively improve ARC’s integration with TCKDB, make remote job handling more efficient, and ensure reliable, isolated test execution.